Bug 2036968: Replaced fast-kde with fftkde and used bootstrap-ci to get CI summary #1034
Conversation
✅ Deploy Preview for mozilla-perfcompare ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1034 +/- ##
==========================================
+ Coverage 96.38% 96.56% +0.17%
==========================================
Files 113 113
Lines 3267 3288 +21
Branches 736 746 +10
==========================================
+ Hits 3149 3175 +26
+ Misses 117 112 -5
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Good start! As discussed, let's switch to echarts for feature, performance and accessibility reasons. I've updated the examples in the other repo so that it is even more drop in, and added a "gallery" with multiple examples. The closest this PR looks like the examples the best it is. In particular, it's going to be a lot less visually noisy, with lots of useless numbers removed. This is a complaint I hear from perf people. lmk if we should do a quick 1:1 one off or pair to get this done! |
Thanks, @padenot. I went ahead and switched to echarts. Should I continue with the rest of the updates or should we break them into separate PRs? I feel this PR is pretty full already. |
padenot
left a comment
There was a problem hiding this comment.
As discussed, those are only some changes to do now, and we'll do the rest in a second PR. Looks nice, most comments are cosmetics or just comments (not linked to any action).
|
Found a bug in my patch. The graph breaks with this comparison on some tests. https://deploy-preview-1034--mozilla-perfcompare.netlify.app/compare-results?baseRev=422aa058a4418b51b7cbccc3a6b3cc0e5f4c5d5b&baseRepo=autoland&newRev=b2144d802d0ad9cf9d259ca0cf62f9ec9b3615ec&newRepo=autoland&framework=13&sort=delta%7Cdesc |
…ow both densities at a single x position Updated tests to reflect changes
f2bb4a7 to
22dafeb
Compare
…man fallback, ref bail-out, and tooltip formatter; also improved x-axis label
Bug fixed. |
padenot
left a comment
There was a problem hiding this comment.
Looks good. Look for follow-up for a couple things we discussed on the channels.
This PR (Bug 2036968) integrates the js version of @padenot's standalone-stats into perfcompare. Here are the main changes for the integration:
Deploy Link
1.) Added
standalone-stats/kde.jsandkde.d.ts;bootstrap-ci.jsandbootstrap-ci.tstosrc/utils2.) In
src/components/CompareResults/CommonGraph.tsx- Removed the thefast-kdeimport and replaced with thefftkde. I removedapproximateSJBandwidthbecause the ISJ method below auto-selects the bandwidth per dataset. It also has an internal grid and quantile logic so didn't need thequantileSortedmethod.3.) Use the
bootstrapMedianDiffCIfrombootstrap-cito get the confidence interval summary and confidence interval statement.Note: please check that the comments are relevant as I may have missed to update or add them where necessary.
Still need to add coverage.
Updates: